Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix spi flash driver support for >16MB devices #9

Closed
wants to merge 1 commit into from
Closed

Fix spi flash driver support for >16MB devices #9

wants to merge 1 commit into from

Conversation

mattwood2000
Copy link

This patch fixes the assumption that 16MB is the upper boundary for
read operations.

Signed-off-by: Matt Wood [email protected]

This patch fixes the assumption that 16MB is the upper boundary for
read operations.

Signed-off-by: Matt Wood <[email protected]>
@ehristev ehristev self-assigned this Aug 16, 2018
@ehristev
Copy link

Is this boundary reached in any situation?
Only the read operations require the boundary to be lifted ?
Which was the original purpose of this boundary ?

@mattwood2000
Copy link
Author

I believe the boundary was incorrectly hard coded to be 16MB for no particular reason. Perhaps at the time 16MB was the device size used to test the driver, but I'm not entirely sure as I am not the original author.

The boundary value is used to calculate how much data is left during a read operation, so clearly setting this to a hard-coded value is is not correct.

I tested this patch using a 32MB device. Read, write, and erase operations were all exercised at address ranges below and above 16 MB as well as at the upper memory boundary of the device.

@ehristev
Copy link

Did you check CONFIG_SPI_FLASH_BAR ?
There is a menu inside u-boot menuconfig configuration for flashes above 16 MiB
As it looks to me, to have flashes higher capacity than 16 MiB , one needs to have more address bytes (4 bytes as it looks in the documentation), and this may be an issue with older flashes, etc.
As there is some support for higher than 16 MiB flashes in U-boot, is this change fixing this driver (namely this is a bug in u-boot core for this flash support ) ?

@mattwood2000
Copy link
Author

Hi Eugen, in looking at the code again, I mistakenly thought the #endif on line 562 was an #else which would have excluded the code when BAR support was enabled. This is extremely puzzling...during my debugging I did test BAR support but got bad data, or so I thought. I re-tested the unmodified branch with BAR support and see read/write operations happening correctly at the 16MB boundary and above.
Now I need to figure out what I was doing wrong the first time!

I will close this pull request. Thanks for the sanity checks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants